Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove x86 macOS runners from the test suite #453

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Remove x86 macOS runners from the test suite #453

merged 3 commits into from
Aug 22, 2024

Conversation

IgorTatarnikov
Copy link
Member

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
torch is no longer supporting x86 macOS, therefore we're dropping it from our suite of tests.

What does this PR do?
Removes x86 macOS runners.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@IgorTatarnikov IgorTatarnikov marked this pull request as ready for review August 21, 2024 15:43
@IgorTatarnikov IgorTatarnikov requested a review from a team August 21, 2024 16:18
@adamltyson
Copy link
Member

Should we also get rid of the macOS x86 dependencies too? It seems weird to keep logic specific to these machines, but then not test it.

@IgorTatarnikov
Copy link
Member Author

I can see benefits for both, leaving the line in will allow macOS x86 users to continue to install cellfinder easily, but with no guarantees of it working for us, but like you said it does feel a bit weird to specify logic but then not test it.

For fun I pulled download numbers by system and cpu and it seems like x86 mac did make up a not insignificant number of downloads (that reported the system/cpu information) but seems to be decreasing fairly drastically over the past year.

For the last year:

project name cpu num_downloads
cellfinder Darwin arm64 354
cellfinder Darwin x86_64 1164
cellfinder Linux ppc64le 2
cellfinder Linux aarch64 2674
cellfinder Linux x86_64 1908
cellfinder Linux armv7l 30
cellfinder Windows AMD64 1540
cellfinder Windows   17
cellfinder     90561

For the past 6 mo:

project name cpu num_downloads
cellfinder Darwin arm64 285
cellfinder Darwin x86_64 183
cellfinder Linux aarch64 2670
cellfinder Linux armv7l 10
cellfinder Linux x86_64 1174
cellfinder Windows AMD64 1040
cellfinder     49214

And 60 days:

project name cpu num_downloads
cellfinder Darwin arm64 66
cellfinder Darwin x86_64 49
cellfinder Linux x86_64 313
cellfinder Linux aarch64 69
cellfinder Linux armv7l 2
cellfinder Windows AMD64 245
cellfinder     17483

@adamltyson
Copy link
Member

I think we should just (unfortunately) stop supporting x86 macs in any way. We can patch things up now, but it will get harder when {other-package} drops support at some point.

@adamltyson adamltyson merged commit f1b7684 into main Aug 22, 2024
17 checks passed
@adamltyson adamltyson deleted the remove-macos13 branch August 22, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamltyson adamltyson adamltyson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@IgorTatarnikov @adamltyson